[DX-1122] Fix: replace flags with positional arguments for primary entity identifiers#361
Merged
[DX-1122] Fix: replace flags with positional arguments for primary entity identifiers#361
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR standardizes the Ably CLI argument style by replacing certain “primary identifier” flags (e.g., --name, --channel, --location) with required positional arguments, aligning sibling commands with docopt/POSIX conventions and updating tests/docs accordingly.
Changes:
- Convert primary entity identifiers from flags to required positional arguments across apps, rules, keys, queues, push channels, and spaces locations commands.
- Update command implementations, help examples/topic index examples, and unit/E2E tests to use the new positional argument forms.
- Adjust shared test helper documentation to reflect the new argument style.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/commands/spaces/spaces.test.ts | Updates spaces command test to pass location as a positional argument. |
| test/unit/commands/spaces/locations/set.test.ts | Updates validation/functionality tests for spaces:locations:set to use positional location. |
| test/unit/commands/queues/create.test.ts | Updates queue create tests to pass queue name positionally and removes --name from flag assertions. |
| test/unit/commands/push/channels/save.test.ts | Updates push channel save tests to pass channel name positionally and drops --channel from flag assertions. |
| test/unit/commands/push/channels/remove.test.ts | Updates push channel remove tests to pass channel name positionally and drops --channel from flag assertions. |
| test/unit/commands/push/channels/remove-where.test.ts | Updates remove-where tests to require channel positional arg and drops --channel from flag assertions. |
| test/unit/commands/push/channels/list.test.ts | Updates list tests to require channel positional arg and drops --channel from flag assertions. |
| test/unit/commands/auth/keys/create.test.ts | Updates key create tests to pass key name positionally and updates missing-arg expectations. |
| test/unit/commands/apps/rules/create.test.ts | Updates rules create tests to pass rule name positionally and drops --name from flag assertions. |
| test/unit/commands/apps/create.test.ts | Updates apps create tests to pass app name positionally and updates missing-arg expectations. |
| test/helpers/standard-tests.ts | Updates helper doc comment example to reflect positional args. |
| test/e2e/spaces/spaces-e2e.test.ts | Updates spaces locations set E2E usage to pass location JSON positionally. |
| test/e2e/push/channels-e2e.test.ts | Updates push channels E2E usage/validation to use positional channel arg. |
| test/e2e/control/control-api-workflows.test.ts | Updates control API workflow E2E sequences to use positional names for create commands. |
| src/commands/spaces/locations/set.ts | Removes --location flag; adds required positional location argument and updates examples/parsing. |
| src/commands/queues/index.ts | Updates topic examples to show queues create with positional queue name. |
| src/commands/queues/create.ts | Replaces required --name flag with required positional queueName argument and updates request construction. |
| src/commands/push/index.ts | Updates topic example for push channels list to use positional channel argument. |
| src/commands/push/channels/save.ts | Replaces required --channel flag with required positional channelName and updates output/logging. |
| src/commands/push/channels/remove.ts | Replaces required --channel flag with required positional channelName and updates confirmation/logging. |
| src/commands/push/channels/remove-where.ts | Replaces required --channel flag with required positional channelName and updates request params/logging. |
| src/commands/push/channels/list.ts | Replaces required --channel flag with required positional channelName and updates request params/logging. |
| src/commands/push/channels/index.ts | Updates push channels topic examples to use positional channel argument. |
| src/commands/auth/keys/index.ts | Updates auth keys topic example to use positional key name. |
| src/commands/auth/keys/create.ts | Replaces required --name flag with required positional keyName and updates logging/request. |
| src/commands/apps/rules/index.ts | Updates apps rules topic examples to use positional rule name for create/update/delete. |
| src/commands/apps/rules/create.ts | Replaces required --name flag with required positional ruleName and updates namespace creation payload. |
| src/commands/apps/create.ts | Replaces required --name flag with required positional appName and updates logging/request. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sacOO7
force-pushed
the
fix/flags-with-arguments
branch
from
1d95035 to
fcd0e4b
Compare
WalkthroughThis PR standardises 9 commands to follow POSIX / docopt CLI conventions by converting Changes
Review Notes
|
![claude-code-ably-assistant[bot]](https://avatars.githubusercontent.com/in/2109912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
The changes are mechanically correct and well-executed. All 9 command conversions follow the same pattern: Args.string() with camelCase name, required: true, and every flags.X reference updated to args.X. Unit tests, E2E tests, and examples are fully in sync.
One pre-existing issue: src/commands/queues/index.ts delete example shows a queue name (my-queue) but queues delete matches on queue ID (format: appAbc:us-east-1-a:foo per its own examples). This was misleading before the PR too, but now that create is corrected, delete stands out. Worth a follow-up fix.
Everything else looks good: camelCase arg names, Flags import correctly removed from spaces/locations/set.ts, standardFlagTests only checks help output so removing the old flags from those lists is safe, error assertions correctly updated to Missing 1 required arg, Copilot comments already addressed in follow-up commit, AGENTS.md updated with new convention. No blocking issues.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sacOO7
force-pushed
the
fix/flags-with-arguments
branch
from
017798b to
a800f0b
Compare
AndyTWF
approved these changes
Apr 22, 2026
sacOO7
changed the title
arguments on commands where the flag value represents the primary entity being acted upon, aligning with CLI conventions (docopt, POSIX, sibling commands).
sacOO7
force-pushed
the
fix/flags-with-arguments
branch
from
a800f0b to
fb9b7f7
Compare
sacOO7
added a commit
to ably/docs
that referenced
this pull request
Apr 22, 2026
… identifiers Reflects ably/ably-cli#361 — converts --name, --channel, and --location flags to positional arguments across 9 command docs: apps create, apps rules create, auth keys create, queues create, push channels list/save/remove/remove-where, and spaces locations set.
umair-ably
added a commit
that referenced
this pull request
Apr 23, 2026
…es/push/locations Align e2e tests with the --name/--channel/--location -> positional-arg refactor from #361.
umair-ably
added a commit
that referenced
this pull request
Apr 23, 2026
…es/push/locations Align e2e tests with the --name/--channel/--location -> positional-arg refactor from #361.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Positional arguments = the thing being acted on (uppercase
NAMEor<name>)Options/flags = modifiers of behavior (
--flag,-f)and If it answers "how should the operation be performed?" it should be a flag.
--name,--channel,--location) for values that represent the primary object being acted upon, while sibling commands correctly use positional arguments for the same purpose.ably apps create --name "APP_NAME"ably apps delete "APP_NAME"ably apps rules create --name "chat" --persistedably apps rules delete "chat"ably queues create --name "my-queue"ably queues delete "my-queue"Commands changed
apps create--nameAPP_NAME(required)apps rules create--nameRULE_NAME(required)auth keys create--nameKEY_NAME(required)queues create--nameQUEUE_NAME(required)push channels list--channelCHANNEL_NAME(required)push channels save--channelCHANNEL_NAME(required)push channels remove--channelCHANNEL_NAME(required)push channels remove-where--channelCHANNEL_NAME(required)spaces locations set--locationLOCATION(required)Commands unchanged
integrations create--rule-type(required)http|amqp|kinesis|...); not a primary entityintegrations create--source-type(required)push config set-fcm--service-account(required)Summary
--name,--channel,--location) to positional argumentswhere the value identifies the primary entity being acted upon, following CLI conventions
(docopt, POSIX §12, sibling command patterns)
REPLACE_FLAGS_WITH_ARGUMENTS.mdfor the full audit and rationaleBreaking changes
These are breaking changes for users passing
--name,--channel, or--location.